Skip to content

Static Code Analysis with Php Inspections (EA Extended) #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 18, 2015
Merged

Static Code Analysis with Php Inspections (EA Extended) #97

merged 5 commits into from
Jul 18, 2015

Conversation

@kalessil
Copy link
Contributor

@kalessil kalessil commented Jul 18, 2015

UTs: passed
Non-functional changes, micro-optimizations

GromNaN
GromNaN reviewed Jul 18, 2015
View reviewed changes
lib/cache/sfMemcacheCache.class.php
Copy link
Collaborator

@GromNaN GromNaN Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct addServer

Copy link
Contributor Author

@kalessil kalessil Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably PhpStoms has incorrect definition in stubs, will revert this.

Copy link
Collaborator

@GromNaN GromNaN Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. 1cb8f45

Copy link
Contributor Author

@kalessil kalessil Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@GromNaN
Copy link
Collaborator

GromNaN commented Jul 18, 2015

Replacing rand by mt_rand seems to be a good idea.

The mt_rand() function [...] will produce random numbers four times faster than what the average libc rand() provides.

GromNaN
GromNaN reviewed Jul 18, 2015
View reviewed changes
lib/config/sfPluginConfiguration.class.php
Copy link
Collaborator

@GromNaN GromNaN Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 http://php.net/manual/en/reflectionclass.getfilename.php

Copy link
Contributor Author

@kalessil kalessil Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this found automatically, I spent couple hours just fixing.

@kalessil
Copy link
Contributor Author

kalessil commented Jul 18, 2015

mt_* is basically new generation replacement. Hopefully I'll find time to write a PHP CS FIxer to automate this (like for is_null, alias functions, __DIR__ usage).

GromNaN
GromNaN reviewed Jul 18, 2015
View reviewed changes
lib/request/sfRequest.class.php
Copy link
Collaborator

@GromNaN GromNaN Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@GromNaN
Copy link
Collaborator

GromNaN commented Jul 18, 2015

Thanks, that's awesome. All that micro-optimizations will be rewarding for all our apps.
I'm curious to know how you did that.

@kalessil
Copy link
Contributor Author

kalessil commented Jul 18, 2015

Static Code Analysis is my tool: https://plugins.jetbrains.com/plugin/7622 (you'll need PhpStorm for it).

And fixing done partially by those fixers:
PHP-CS-Fixer/PHP-CS-Fixer#1304
PHP-CS-Fixer/PHP-CS-Fixer#1302
PHP-CS-Fixer/PHP-CS-Fixer@d86a8aa

Quite lot of this is automated (not everything of course).

@kalessil
Copy link
Contributor Author

kalessil commented Jul 18, 2015

SCA Tool is advanced one, but most probably you'll want to adjust it for your needs.

Feel free to ask questions )

GromNaN added a commit that referenced this pull request Jul 18, 2015
@GromNaN
Merge pull request #97 from kalessil/master
c85e512 
Static Code Analysis with Php Inspections (EA Extended)
@GromNaN GromNaN merged commit c85e512 into FriendsOfSymfony1:master Jul 18, 2015
@GromNaN
Copy link
Collaborator

GromNaN commented Jul 18, 2015

Thanks @kalessil. I'll try it on my projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kalessil @GromNaN